Treat rich text editor newlines as line breaks, not paragraph breaks#10293
Treat rich text editor newlines as line breaks, not paragraph breaks#10293alokedesai wants to merge 8 commits into
Conversation
…tory - Add RichTextStylesExt trait with new_with_default_line_height for comment editors - Add IndentableBlockSpacing::new() and BlockSpacings::uniform() constructors - Add style_factory field to RichTextEditorConfig/RichTextEditorView - Consolidate style_factory with initial styles in comment_editor - Compact text block spacings, 3px cursor width, remove minimum_paragraph_height Co-Authored-By: Oz <oz-agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds custom rich text styling for code review comment editors, wires a style factory through RichTextEditorView so styles rebuild correctly after appearance/font changes, and adds related editor keybindings.
Concerns
- No blocking correctness or security concerns found.
- Two comment typos were introduced in touched code.
Verdict
Found: 0 critical, 0 important, 2 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| EditableBinding::new( | ||
| // This doesn't reuse the move_to_line_start naming from the terminal input editor to | ||
| // distinguish between soft-wrapped line and hard-wrapped line (paragraph) movement. | ||
| // distinguish between soft-wrappped line and hard-wrapped line (paragraph) movement. |
There was a problem hiding this comment.
🧹 [NIT] Fix the typo in this comment.
| // distinguish between soft-wrappped line and hard-wrapped line (paragraph) movement. | |
| // distinguish between soft-wrapped line and hard-wrapped line (paragraph) movement. |
| } | ||
|
|
||
| // Represents the states of an ongoing mouse event. Note that these states are mutually exclusive: | ||
| // Represents the states of an ongoing mouse event. Note that these states are mutally exclusive: |
There was a problem hiding this comment.
🧹 [NIT] Fix the typo in this comment.
| // Represents the states of an ongoing mouse event. Note that these states are mutally exclusive: | |
| // Represents the states of an ongoing mouse event. Note that these states are mutually exclusive: |
Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
alokedesai
changed the title
1 participant
Description
The rich text editor would treat each newline as a paragraph break instead of line break causing the user-visible line height to look unusually large.
This PR changes the styling of the rich text editor to more closely mimic a normal markdown editor instead of an editor like Notion.
Specifically:
0px. There's now no difference in padding between hard-wrapped and soft-wrapped lines.post_processnotebook step that strips newlines. This processing step was just wrong, we should use the filesystem as the source of truth and it would remove multiple newlines between content (foo\n\n\n\nbarwould becomefoo\nbar).Testing
I did a bunch of monkey testing with AI-generated markdown. See screenshots and looms below